Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial findings and updates for hoard #1

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

cwholt
Copy link

@cwholt cwholt commented Oct 17, 2011

hoard is pretty sweet. my initial findings are that it was a bit broken, so i fixed a few things:

added a .gitignore file

README.md

  • update -> updateMany
  • comments about testing

test/hoard.test.coffee

  • updated reference to hoard.js location (in lib/)

src/hoard.coffee

  • out-of-the-box failing due to propagateLowerArchives being defined after it is called (is this a compiler specific issue?)
  • copied propagateLowerArchives code from .updateManyArchive to .update with a small addition (like i said in comments, not an ideal implementation but just wanted to get working)
  • try/catch for handling race condition with fs.read when offset is out of bounds (doesn't fix it, just handles it).

i am doing significant load testing using this library, and i fear there are many other exceptions that need to be handled, and i will try and fix, or at least handle, them as they present themselves. so far it's doing about 2000 requests per minute no problem, but when hitting the same hoard file there are periodic errors.

i also played around with adding a .last function to get the last value of the highest resolution archive, but it's not ideal so i reverted it.

thanks for the great library!

@cgbystrom
Copy link
Owner

Cool, thanks for contributing your patches back. I don't have the time right now to review and merge your pull request but I will, just a lot things going on at work.

@cwholt
Copy link
Author

cwholt commented Oct 21, 2011

no worries, you can disregard this until you're ready to check it out. i'll continue moving through getting it up to speed and working, maybe building out the tests a bit more as well.

@ciaranj
Copy link

ciaranj commented Oct 15, 2012

I've also submitted a pull request on-top-of this one that adds some more fixes, and support for aggregation method types. ( #4 )

@jnovack
Copy link

jnovack commented Mar 11, 2014

I want to dive into hoard and get rid of shelling out to rrdtool for a few projects, but am not sure who's branch to go with.

  • has there been any stability progress on this project (or on any of the forks) or has it not needed it?
  • despite being 3 years old (60 in nodejs years), is this relatively stable and solid? @cwholt I assume you have been using it for 2 years, is it good?
  • are there different better libraries out there within nodejs for timeseries or is this the standard?

@cgbystrom, if you have abandoned this project, can you kindly release it (including the npm package) to allow someone who will integrate all the fixes and patches? Either @cwholt, @ciaranj or myself if neither are available. Should only take someone a few hours to version bump this and organize it.

@cwholt
Copy link
Author

cwholt commented Mar 11, 2014

unfortunately i am no longer using it for any projects, but the project that was using it is still running strong (and actually has not been touched for 2 years). I left that company and am now using Graphite w/ StatsD (in hindsight, a better choice).

@jnovack
Copy link

jnovack commented Mar 11, 2014

cwholt: I was looking into that, and I understand the scaling cap I'll run into. I didn't want to have to install a massive graphite installation for this tiny project. If you could leave your repo open, until we review and merge, that would be helpful. Thank you.

cgbystrom: I definitely want to use this, but now my OCD wants to make sure that npm has been updated so others could benefit. It is definitely getting some use out there. :)

@ciaranj
Copy link

ciaranj commented Mar 11, 2014

So I fixed a small amount of issues in the hoard library, but actually due to the disk IO requirements it had, I ended up porting the newer ceres-db (which replaces whisper in mega-carbon) to node.js, and added a backend to statsd that utilised that instead. ...and then did a rough port of the awesome graphite to node.js too (still a WiP but I'm using it in production reasonably successfully currently.)

http://chartled.com/
https://github.com/ciaranj/ceres/tree/nodejs_port
https://github.com/ciaranj/statsd/tree/add_ceres_backend

It seems to work (for me) considerably better than I ever got hoard to (in terms of Disk IO at least!)

As @cwholt says, the official graphite/statsd combination will likely be the best bet for anything you do, but if you do want to stay 'pure' js (I have my reasons :/ ) then my stuff may at least send you on the right (or a more right) path.

If you are interested in trying a set up, drop me a line and I'll send you the intstructions (due to the branches etc. it isn't all just npm install -d ;) )

@cgbystrom
Copy link
Owner

Sure, I'll gladly transfer hoard package to anyone who wants to continue maintaining it.

It seems as @cwholt and @ciaranj no longer use/maintain branches of it. So that leaves you :) @jnovack, what's your username on npm?

@jnovack
Copy link

jnovack commented Mar 12, 2014

npm profile

Thank you for your hard work on the project.

Will you be transferring the repository as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants